Skip to content

Conversation

@scottnemes
Copy link
Contributor

Description

What:
Reworks the reconnect logic (both the command \r and calls from other parts of the code) to actually create a new database connection instead of simply running a command to change the database.

Why:
With the existing reconnect logic, some people would get long reconnect times. Plus the logic to reconnect was more indirect as it was a result of calling another command instead of specifically creating a new connection.

Resolves #746

Checklist

  • I've added this contribution to the changelog.md.
  • I've added my name to the AUTHORS file (or it's already there).
  • I ran uv run ruff check && uv run ruff format && uv run mypy --install-types . to lint and format the code.

… for use by the command \r to call the new reconnect function. Updated help output in tests to match the change.
… for use by the command \r to call the new reconnect function. Updated help output in tests to match the change.
…emes/mycli into feat/746/rework-reconnect-command
special.register_special_command(self.change_db, "use", "\\u", "Change to a new database.", aliases=["\\u"])
special.register_special_command(
self.change_db,
"connect",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the full command from "connect" to "reconnect" is a needless breaking change for the user. Maybe it is more rational since both the long and short versions start with "r"? We should be able to add "reconnect" as an alias here:

 aliases=["\\r", "reconnect"],

if that is the motivation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I switched it back to connect. Goal was consistency, but your point makes sense and it matches the official client anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And RE: adding the alias, I pondered it but decided to leave it off for now. Theoretically in some future change we could split it out to actually be "connect" and "reconnect" with different logic (may never happen, but possible) so didn't want to further muddy the waters there.

Copy link
Contributor

@rolandwalker rolandwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@rolandwalker rolandwalker merged commit be626f8 into dbcli:main Dec 29, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make reconnect command create a new connection to the database

2 participants